[v12.x] deps: V8: backport 07ee86a5a28b #32619

miladfarca · 2020-04-02T17:09:33Z

Original commit message:

PPC: allow for calling CFunctions without function descriptors on AIX.

The calling conventions on AIX uses function descriptors,
which means that pointers to functions do not point to code,
but instead point to metadata about them. When calling JITed code,
we must assure to use function descriptors instead of raw pointers when
needed. Before this CL 213504b, all CallCFunction on AIX were guaranteed to have
function descriptors. Starting form the CL mentioned above, CallCFunction can also
Jump to a Trampoline which does not have a function descriptor, hence a new
"CallCFunctionWithoutFunctionDescriptor" method is proposed to deal with this issue.

BUG= v8:9766

Change-Id: I9343c31c812f5d4dda8503a5adf024b24dbde072
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1825961
Commit-Queue: Milad Farazmand <[email protected]>
Reviewed-by: Michael Starzinger <[email protected]>
Reviewed-by: Jakob Gruber <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64357}

Refs: v8/v8@07ee86a

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Original commit message: PPC: allow for calling CFunctions without function descriptors on AIX. The calling conventions on AIX uses function descriptors, which means that pointers to functions do not point to code, but instead point to metadata about them. When calling JITed code, we must assure to use function descriptors instead of raw pointers when needed. Before this CL 213504b, all CallCFunction on AIX were guaranteed to have function descriptors. Starting form the CL mentioned above, CallCFunction can also Jump to a Trampoline which does not have a function descriptor, hence a new "CallCFunctionWithoutFunctionDescriptor" method is proposed to deal with this issue. BUG= v8:9766 Change-Id: I9343c31c812f5d4dda8503a5adf024b24dbde072 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1825961 Commit-Queue: Milad Farazmand <[email protected]> Reviewed-by: Michael Starzinger <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64357} Refs: v8/v8@07ee86a

nodejs-github-bot · 2020-04-02T19:57:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30385/

richardlau

RSLGTM. I've added a commit to undo the skip of the --jitless test on AIX from #32594.

…o aix-fd-patch

miladfarca · 2020-04-02T20:27:33Z

@richardlau Could you please start another pr test

mhdawson

Rubber Stamp LGTM

nodejs-github-bot · 2020-04-02T21:13:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30386/

richardlau · 2020-04-02T21:16:35Z

@richardlau Could you please start another pr test

GitHub has been a bit unstable this evening. Will wait a bit for things to stabilise.

miladfarca · 2020-04-02T21:17:26Z

@richardlau sounds good, seems like one of my commits still hasn't showed up.

bnoordhuis · 2020-04-02T21:31:31Z

deps/v8/src/common/globals.h

@@ -404,6 +404,7 @@ enum TypeofMode : int { INSIDE_TYPEOF, NOT_INSIDE_TYPEOF };
 // Enums used by CEntry.
 enum SaveFPRegsMode { kDontSaveFPRegs, kSaveFPRegs };
 enum ArgvMode { kArgvOnStack, kArgvInRegister };
+enum FunctionDescriptorMode { kNoFunctionDescriptor, kHasFunctionDescriptor };


Shouldn't this have been enum FunctionDescriptorMode : bool { /* ... */ }, or (probably more idiomatic for V8) use the enum instead of bool? Relying on int-to-bool coercion looks a bit footgunny.

I'm guessing they were ok with it as it's following the previous enums SaveFPRegsMode and ArgvMode.

Right, but those are of type SaveFPRegsMode and ArgvMode in function parameter lists, there's no coercion to bool taking place.

that's right however the int value 0 (kNoFunctionDescriptor) is implicitly translating to false at all cases, and true otherwise. You're right that marking it as bool could have made it more clear.

MylesBorins · 2020-04-02T21:40:53Z

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/2994/

miladfarca · 2020-04-02T22:08:34Z

Should be safe to run tests now, files are synced.

miladfarca · 2020-04-02T22:23:08Z

FYI, implementation under src/compiler/backend/ppc/code-generator-ppc.cc and src/execution/simulator.h had to be slightly modified from the original CL as certain changes are not present in this version if V8.

nodejs-github-bot · 2020-04-03T08:22:38Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30415/
V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/2997/

Original commit message: PPC: allow for calling CFunctions without function descriptors on AIX. The calling conventions on AIX uses function descriptors, which means that pointers to functions do not point to code, but instead point to metadata about them. When calling JITed code, we must assure to use function descriptors instead of raw pointers when needed. Before this CL 213504b, all CallCFunction on AIX were guaranteed to have function descriptors. Starting form the CL mentioned above, CallCFunction can also Jump to a Trampoline which does not have a function descriptor, hence a new "CallCFunctionWithoutFunctionDescriptor" method is proposed to deal with this issue. BUG= v8:9766 Change-Id: I9343c31c812f5d4dda8503a5adf024b24dbde072 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1825961 Commit-Queue: Milad Farazmand <[email protected]> Reviewed-by: Michael Starzinger <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64357} Refs: v8/v8@07ee86a PR-URL: #32619 Refs: v8/v8@07ee86a Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>

PR-URL: #32619 Refs: v8/v8@07ee86a Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>

MylesBorins · 2020-04-03T20:11:44Z

landed in e88960d...e685f12

nodejs-github-bot added build Issues and PRs related to build files or the CI. v12.x v8 engine Issues and PRs related to the V8 dependency. labels Apr 2, 2020

miladfarca mentioned this pull request Apr 2, 2020

[v12.x] --jitless not working on aix on v12.x #32600

Closed

targos changed the title ~~deps: V8: backport 07ee86a5a28b~~ [v12.x] deps: V8: backport 07ee86a5a28b Apr 2, 2020

test: restore --jitless test on AIX

66d149f

richardlau approved these changes Apr 2, 2020

View reviewed changes

Milad Farazmand added 2 commits April 2, 2020 20:13

Adding #ifdef termination

2738b0c

Merge branch 'aix-fd-patch' of https://github.com/miladfarca/node int…

791d215

…o aix-fd-patch

mhdawson approved these changes Apr 2, 2020

View reviewed changes

bnoordhuis approved these changes Apr 2, 2020

View reviewed changes

Milad Farazmand added 2 commits April 2, 2020 22:06

sync files

ce31a68

remove change after file sync

b5b0331

This comment has been minimized.

Sign in to view

MylesBorins closed this Apr 3, 2020

codebytere mentioned this pull request Apr 4, 2020

v12.16.2 proposal #32313

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v12.x] deps: V8: backport 07ee86a5a28b #32619

[v12.x] deps: V8: backport 07ee86a5a28b #32619

miladfarca commented Apr 2, 2020 •

edited

Loading

nodejs-github-bot commented Apr 2, 2020

richardlau left a comment

miladfarca commented Apr 2, 2020

mhdawson left a comment

nodejs-github-bot commented Apr 2, 2020

richardlau commented Apr 2, 2020

miladfarca commented Apr 2, 2020

bnoordhuis Apr 2, 2020

miladfarca Apr 2, 2020

bnoordhuis Apr 2, 2020

miladfarca Apr 2, 2020

MylesBorins commented Apr 2, 2020

miladfarca commented Apr 2, 2020

This comment has been minimized.

miladfarca commented Apr 2, 2020

nodejs-github-bot commented Apr 3, 2020 •

edited by MylesBorins

Loading

MylesBorins commented Apr 3, 2020

[v12.x] deps: V8: backport 07ee86a5a28b #32619

[v12.x] deps: V8: backport 07ee86a5a28b #32619

Conversation

miladfarca commented Apr 2, 2020 • edited Loading

Checklist

nodejs-github-bot commented Apr 2, 2020

richardlau left a comment

Choose a reason for hiding this comment

miladfarca commented Apr 2, 2020

mhdawson left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Apr 2, 2020

richardlau commented Apr 2, 2020

miladfarca commented Apr 2, 2020

bnoordhuis Apr 2, 2020

Choose a reason for hiding this comment

miladfarca Apr 2, 2020

Choose a reason for hiding this comment

bnoordhuis Apr 2, 2020

Choose a reason for hiding this comment

miladfarca Apr 2, 2020

Choose a reason for hiding this comment

MylesBorins commented Apr 2, 2020

miladfarca commented Apr 2, 2020

This comment has been minimized.

miladfarca commented Apr 2, 2020

nodejs-github-bot commented Apr 3, 2020 • edited by MylesBorins Loading

MylesBorins commented Apr 3, 2020

miladfarca commented Apr 2, 2020 •

edited

Loading

nodejs-github-bot commented Apr 3, 2020 •

edited by MylesBorins

Loading